-
Notifications
You must be signed in to change notification settings - Fork 32
Backport index and ipni pdp tasks to PDPv0 #670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: pdpv0
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some DB changes are required to make this work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This task will only produce IPFS style ads. So, there would be no IPNI lookup path for pieces. Which was a requirement confirmed by Jen and Miro both in Dubai.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 That's fine for now. I just want to get ipfs adds working first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
piece CIDs into IPNI is lower priority; if it can be bundled into the same work without too much effort then that's great, but it doesn't buy us very much so it can be treated as a stretch goal
@ZenGround0 : will https://github.com/filecoin-project/curio/blob/feat/index-and-ipni/deps/config/types.go#L116 be updated to include filecoinpin.contact, or will that be somewhere else? |
tasks/indexing/task_pdp_indexing.go
Outdated
select { | ||
case recs <- indexstore.Record{ | ||
Cid: blockMetadata.Cid, | ||
Offset: blockMetadata.Offset, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Offset: blockMetadata.Offset, | |
Offset: blockMetadata.SourceOffset, |
See https://pkg.go.dev/github.com/ipld/go-car/v2#BlockMetadata, this is important to support CARv2 which has a header before the internal CARv1 container.
But I see this is just copied straight from tasks/indexing/task_indexing.go and the same use exists above in tasks/indexing/task_pdp_ipni.go and where it's copied from in tasks/indexing/task_ipni.go. So @LexLuthr you're going to need to fix them too.
The Boost version of this is a good reference fwiw, (that you were responsible for @LexLuthr): filecoin-project/boost#1739 (see piecedirectory/piecedirectory.go parseRecordsFromCar
).
I also note that we're not supporting PoDSI here; something to look into later maybe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#681 fixes it for main.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we filter out cid.contract for anything not mainnet. We should remove that check as well.
Backporting the indexing and ipni tasks from mkv2 to the pdpv0 branch.
Code is at least not obviously broken but testing is a work in progress.